static-delta: add some error handling

We make _ostree_parse_delta_name() a bit more defensive since it handles
user input.

Closes: #504
Closes: #505
Approved by: cgwalters

diff --git a/src/libostree/ostree-core-private.h b/src/libostree/ostree-core-private.h
index cb4d953d549dea322a6c20fcd43b7a28b928e134..0c5fb0eb463d5c2ab77fb5e8df6c1676f1d153cf 100644 (file)
--- a/src/libostree/ostree-core-private.h
+++ b/src/libostree/ostree-core-private.h
@@ -121,10 +121,11 @@ static inline char * _ostree_get_commitpartial_path (const char *checksum)
   return g_strconcat ("state/", checksum, ".commitpartial", NULL);
 }
 
-void
+gboolean
 _ostree_parse_delta_name (const char  *delta_name,
                           char        **out_from,
-                          char        **out_to);
+                          char        **out_to,
+                          GError      **error);
 
 void
 _ostree_loose_path (char              *buf,

diff --git a/src/libostree/ostree-core.c b/src/libostree/ostree-core.c
index 6e3c4e2e79af03278b1bbdba6e7b4155bdfbe970..e3f0a7716d20fb095cc1b1e543709ada6aec5d42 100644 (file)
--- a/src/libostree/ostree-core.c
+++ b/src/libostree/ostree-core.c
@@ -1584,12 +1584,26 @@ _ostree_get_relative_static_delta_part_path (const char        *from,
   return _ostree_get_relative_static_delta_path (from, to, partstr);
 }
 
-void
-_ostree_parse_delta_name (const char  *delta_name,
+gboolean
+_ostree_parse_delta_name (const char   *delta_name,
                           char        **out_from,
-                          char        **out_to)
+                          char        **out_to,
+                          GError      **error)
 {
-  g_auto(GStrv) parts = g_strsplit (delta_name, "-", 2);
+  g_auto(GStrv) parts = NULL;
+  g_return_val_if_fail (delta_name != NULL, FALSE);
+
+  parts = g_strsplit (delta_name, "-", 2);
+
+  /* NB: if delta_name is "", parts[0] is NULL, but the error
+   * validate_checksum_string() gives for "" is nice enough,
+   * so we just coerce it here */
+  if (!ostree_validate_checksum_string (parts[0] ?: "", error))
+    return FALSE;
+
+  if (parts[0] && parts[1] &&
+      !ostree_validate_checksum_string (parts[1], error))
+    return FALSE;
 
   *out_from = *out_to = NULL;
   if (parts[0] && parts[1])
@@ -1601,6 +1615,8 @@ _ostree_parse_delta_name (const char  *delta_name,
     {
       ot_transfer_out_value (out_to, &parts[0]);
     }
+
+  return TRUE;
 }
 
 /*

diff --git a/src/libostree/ostree-repo-static-delta-core.c b/src/libostree/ostree-repo-static-delta-core.c
index 46fa5f8664daf2bc0cf2b185bf15b2774651caf7..82c80a2abade5884da4664e6dfa956d91fa6742c 100644 (file)
--- a/src/libostree/ostree-repo-static-delta-core.c
+++ b/src/libostree/ostree-repo-static-delta-core.c
@@ -795,7 +795,9 @@ _ostree_repo_static_delta_delete (OstreeRepo                    *self,
   g_autofree char *deltadir = NULL;
   struct stat buf;
 
-  _ostree_parse_delta_name (delta_id, &from, &to);
+  if (!_ostree_parse_delta_name (delta_id, &from, &to, error))
+    goto out;
+
   deltadir = _ostree_get_relative_static_delta_path (from, to, NULL);
 
   if (fstatat (self->repo_dir_fd, deltadir, &buf, 0) != 0)
@@ -830,7 +832,9 @@ _ostree_repo_static_delta_query_exists (OstreeRepo                    *self,
   g_autofree char *superblock_path = NULL;
   struct stat stbuf;
 
-  _ostree_parse_delta_name (delta_id, &from, &to);
+  if (!_ostree_parse_delta_name (delta_id, &from, &to, error))
+    return FALSE;
+
   superblock_path = _ostree_get_relative_static_delta_superblock_path (from, to);
 
   if (fstatat (self->repo_dir_fd, superblock_path, &stbuf, 0) < 0)
@@ -854,7 +858,7 @@ gboolean
 _ostree_repo_static_delta_dump (OstreeRepo                    *self,
                                 const char                    *delta_id,
                                 GCancellable                  *cancellable,
-                                GError                      **error)
+                                GError                       **error)
 {
   gboolean ret = FALSE;
   g_autofree char *from = NULL; 
@@ -868,7 +872,9 @@ _ostree_repo_static_delta_dump (OstreeRepo                    *self,
   OstreeDeltaEndianness endianness;
   gboolean swap_endian = FALSE;
 
-  _ostree_parse_delta_name (delta_id, &from, &to);
+  if (!_ostree_parse_delta_name (delta_id, &from, &to, error))
+    goto out;
+
   superblock_path = _ostree_get_relative_static_delta_superblock_path (from, to);
 
   if (!ot_util_variant_map_at (self->repo_dir_fd, superblock_path,

diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c
index 59bfbf9e71a580aa7ced7a28f43e1c1881009f17..e4e1ecbf5e97ffbc0263c1d408b8328d59bde132 100644 (file)
--- a/src/libostree/ostree-repo.c
+++ b/src/libostree/ostree-repo.c
@@ -4701,7 +4701,9 @@ ostree_repo_regenerate_summary (OstreeRepo     *self,
         glnx_fd_close int superblock_file_fd = -1;
         g_autoptr(GInputStream) in_stream = NULL;
 
-        _ostree_parse_delta_name (delta_names->pdata[i], &from, &to);
+        if (!_ostree_parse_delta_name (delta_names->pdata[i], &from, &to, error))
+          goto out;
+
         superblock = _ostree_get_relative_static_delta_superblock_path ((from && from[0]) ? from : NULL, to);
         superblock_file_fd = openat (self->repo_dir_fd, superblock, O_RDONLY | O_CLOEXEC);
         if (superblock_file_fd == -1)

diff --git a/tests/test-checksum.c b/tests/test-checksum.c
index 9537804da30e5d450999f0da73578085d2e618c3..7d2a0ceae79a9b8191208a0018d2266f08b63c86 100644 (file)
--- a/tests/test-checksum.c
+++ b/tests/test-checksum.c
@@ -31,25 +31,49 @@ static void
 test_ostree_parse_delta_name (void)
 {
   {
-    g_autofree char *from;
-    g_autofree char *to;
-    _ostree_parse_delta_name ("30d13b73cfe1e6988ffc345eac905f82a18def8ef1f0666fc392019e9eac388d", &from, &to);
+    g_autofree char *from = NULL;
+    g_autofree char *to = NULL;
+    g_assert (_ostree_parse_delta_name ("30d13b73cfe1e6988ffc345eac905f82a18def8ef1f0666fc392019e9eac388d", &from, &to, NULL));
     g_assert_cmpstr (to, ==, "30d13b73cfe1e6988ffc345eac905f82a18def8ef1f0666fc392019e9eac388d");
     g_assert_null (from);
   }
 
   {
-    g_autofree char *from;
-    g_autofree char *to;
-    _ostree_parse_delta_name ("30d13b73cfe1e6988ffc345eac905f82a18def8ef1f0666fc392019e9eac388d-5891b5b522d5df086d0ff0b110fbd9d21bb4fc7163af34d08286a2e846f6be03", &from, &to);
+    g_autofree char *from = NULL;
+    g_autofree char *to = NULL;
+    g_assert (_ostree_parse_delta_name ("30d13b73cfe1e6988ffc345eac905f82a18def8ef1f0666fc392019e9eac388d-5891b5b522d5df086d0ff0b110fbd9d21bb4fc7163af34d08286a2e846f6be03", &from, &to, NULL));
     g_assert_cmpstr (from, ==, "30d13b73cfe1e6988ffc345eac905f82a18def8ef1f0666fc392019e9eac388d");
     g_assert_cmpstr (to, ==, "5891b5b522d5df086d0ff0b110fbd9d21bb4fc7163af34d08286a2e846f6be03");
   }
 
   {
-    g_autofree char *from;
-    g_autofree char *to;
-    _ostree_parse_delta_name ("", &from, &to);
+    g_autofree char *from = NULL;
+    g_autofree char *to = NULL;
+    g_assert (!_ostree_parse_delta_name ("", &from, &to, NULL));
+    g_assert_null (from);
+    g_assert_null (to);
+  }
+
+  {
+    g_autofree char *from = NULL;
+    g_autofree char *to = NULL;
+    g_assert (!_ostree_parse_delta_name ("GARBAGE", &from, &to, NULL));
+    g_assert_null (from);
+    g_assert_null (to);
+  }
+
+  {
+    g_autofree char *from = NULL;
+    g_autofree char *to = NULL;
+    g_assert (!_ostree_parse_delta_name ("GARBAGE-5891b5b522d5df086d0ff0b110fbd9d21bb4fc7163af34d08286a2e846f6be03", &from, &to, NULL));
+    g_assert_null (from);
+    g_assert_null (to);
+  }
+
+  {
+    g_autofree char *from = NULL;
+    g_autofree char *to = NULL;
+    g_assert (!_ostree_parse_delta_name ("30d13b73cfe1e6988ffc345eac905f82a18def8ef1f0666fc392019e9eac388d-GARBAGE", &from, &to, NULL));
     g_assert_null (from);
     g_assert_null (to);
   }

diff --git a/tests/test-delta.sh b/tests/test-delta.sh
index f820965113c579cf9f57b2595e1a5de6f423f486..bb523b47f6c7d9f165ee9c51c5d67d00dcde03b7 100755 (executable)
--- a/tests/test-delta.sh
+++ b/tests/test-delta.sh
@@ -26,7 +26,7 @@ skip_without_user_xattrs
 bindatafiles="bash true ostree"
 morebindatafiles="false ls"
 
-echo '1..10'
+echo '1..11'
 
 mkdir repo
 ${CMD_PREFIX} ostree --repo=repo init --mode=archive-z2
@@ -235,3 +235,10 @@ ${CMD_PREFIX} ostree --repo=repo2 fsck
 ${CMD_PREFIX} ostree --repo=repo2 ls ${samerev} >/dev/null
 
 echo 'ok pull empty delta part'
+
+if ${CMD_PREFIX} ostree --repo=repo static-delta show GARBAGE 2> err.txt; then
+    assert_not_reached "static-delta show GARBAGE unexpectedly succeeded"
+fi
+assert_file_has_content err.txt "Invalid rev 'GARBAGE'"
+
+echo 'ok handle bad delta name'